Underwriter simplification: consolidate duplication, vectorize bootstrap, generalize ceiling#10
Open
ree2raz wants to merge 1 commit into
Open
Underwriter simplification: consolidate duplication, vectorize bootstrap, generalize ceiling#10ree2raz wants to merge 1 commit into
ree2raz wants to merge 1 commit into
Conversation
…generalize ceiling Behavior-preserving cleanup (all 83 scoring tests pass; bootstrap_ci proven numerically identical to the old loop on 50 random cases). Library code: - bootstrap_ci: 1000-iter Python loop -> one numpy resample matrix (row-major draws consume the RNG stream identically, so CI values are unchanged). - bootstrap_index: hoist loop-invariant np.asarray conversions out of the loop. - price(): replace the hardcoded `if ax == "bias"` pair-divergence branch with a per-axis _ceiling_candidates() generator; adding a new secondary ceiling signal is now one line. Also fixes a latent KeyError when pair_divergence was binding. - _synthetic_axis_risks(): one helper for the (ci_low,risk,ci_high) stand-in that was built in both price() and aggregate_model(). - consensus_verdict(): promoted to public; runner's two re-inlined risk->verdict ternaries now call it (one source of truth for the cutoff). - combine(): reuse the has_hard_leak already computed in the sensitive branch. - runner: extract _oss_backend_or_none() + _ping_oss(), removing the triplicated OSS guard and duplicated ping; drop redundant `settings as s` re-import. Build scripts: - _git_sha (x7), _hf_commit_sha (x6), _download (x3) were copy-pasted verbatim -> scripts/_common.py, alias-imported so call sites are unchanged. Drop 4 now-unused subprocess imports. Net -169 lines across tracked files.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Behavior-preserving cleanup pass over the underwriter module (a
/simplifyreview with four cleanup angles — reuse, simplification, efficiency, altitude). Net −169 lines across tracked files; all 83 scoring tests pass.Verification
pytest underwriter/tests→ 83/83 pass.bootstrap_ciproven numerically identical to the old loop on 50 random cases (row-majorrng.integers(size=(iters,n))consumes the RNG stream in the same order as N sequential draws), so published CI values do not move._common.Library code (test-covered)
bootstrap_ci— 1000-iter Python loop → one numpy resample matrix. Same draws, no per-iteration interpreter overhead.bootstrap_index— hoisted the loop-invariantnp.asarrayconversions out of the hot loop (exact same RNG draws).price()ceiling generalized — the hardcodedif ax == "bias"pair-divergence branch became a per-axis_ceiling_candidates()generator. Adding a new secondary ceiling signal (e.g. sensitivehard_leak_rate) is now one line, not a second special-case. Side benefit: removes a latentKeyErrorwhen pair-divergence was the binding constraint._synthetic_axis_risks()— one helper for the(ci_low, risk, ci_high)stand-in that was built in bothprice()andaggregate_model().consensus_verdict()promoted to public; the risk→verdict ternary that was re-inlined (in opposite branch order) at two runner sites now calls it — one source of truth for the pricing-relevant cutoff.combine()— reuse thehas_hard_leakalready computed in the sensitive branch instead of recomputing + redundantbool()wrap._oss_backend_or_none()+_ping_oss(), removing the triplicatedmodal_oss_url/provider guard and the byte-identical_ping; dropped the redundantfrom .config import settings as sre-import.**{k: v for k, v in raw.items()}→**raw.Build scripts
_git_sha(×7),_hf_commit_sha(×6),_download(×3) were copy-pasted verbatim → newscripts/_common.py, alias-imported so call sites are unchanged. Removed 4 now-unusedsubprocessimports.Deliberately skipped (follow-ups)
combine+tail_risk+aggregate_axisinto one registry. Highest architectural value, but a scoring-semantics redesign too large to land behavior-preserving here._generate()unification inrunner.py— real, but runner has no unit coverage and drives the live eval; not worth the risk without an integration test.load_cardsdouble-YAML-parse — runs once at end of a multi-hour run; negligible impact, and the clean fix adds caching semantics.